Automated review note — AI-generated (Claude), validated against the live diff. Please sanity-check before acting.

Reviewed at head 58da5abb71 — first review of this PR by this workflow. Full report: AIReview, 2026-09-10.

Verdict: REQUEST CHANGES — one blocker in the new FC-import workflow, plus a batch of parameter-name fixes that are cheap and mechanical.

Blocker: the FC-import path uses the wrong "what is non-default" baseline

data_model_vehicle_project.py:306 and :313-315.

_complete_imported_vehicle_project_creation() calls self._local_filesystem.re_init(new_path, vehicle_type) at :306, and re_init() unconditionally reloads param_default_dict from the new vehicle directory (backend_filesystem.py:134) — which has just received a verbatim copy of the template's 00_default.param (it is excluded from the transform, not from the copy). So by the time :314 reads param_default_dict, the connected FC's real defaults, loaded earlier by initialize_filesystem(), have been silently overwritten by the template's.

Reproduced end-to-end with a real LocalFilesystem and the shipped empty_4.7.x template, feeding a mock FC whose parameters are exactly the template's own defaults — i.e. a flight controller at factory settings with nothing configured:

param_default_dict before creation: 1086
67_imported_flight_controller_parameters.param: 181 parameters exported
    ANGLE_MAX,3000   ARMING_CHECK,1   ATC_ACCEL_P_MAX,110000   ATC_SLEW_YAW,6000  ...
param_default_dict after creation: 1090          <- the template's, not the FC's
00_default.param in new project == FC defaults? False

Proved by mutation. Inserting if default_params is None: default_params = self._local_filesystem.param_default_dict.deep_copy() before the re_init() call and re-running the identical scenario gives 1086 after creation, a matching 00_default.param, and no import file at all.

The exact count is template-dependent — a second reviewer measured 65 using diatone_taycan_mxc/4.6.x-params, against a correct baseline of zero in both cases. Across your own board templates the spread is 103–225 (Holybro_X500_V2 138, Big_Owl 119, X11_plus 103, FETtec-5 103, Hoverit_X13 225).

The same root cause also loses genuine settings, which is the worse direction. With SERIAL5_BAUD set to 57 on the FC against that FC's default of 115, the parameter is omitted from every numbered configuration file — because 57 happens to equal the template's default. So the wrong baseline both invents configuration that was never made and silently drops configuration that was.

Why the tests miss it: tests/test_data_model_vehicle_project.py:343-395 uses MagicMock(spec=LocalFilesystem), so the mocked re_init never reloads param_default_dict and the ordering is invisible.

Suggested fix: snapshot param_default_dict (or pass the FC defaults in explicitly) before re_init(), and call write_param_default_values_to_file() on the FC path as the bin-log path already does.

Ten new autoimport patterns use 4.6 names renamed in 4.7

configuration_steps_ArduCopter.json lines 369, 398, 468, 485, 663, 986, 1017, 1051 and 1332 (1332 carries two). Each was matched with re.match, as _apply_auto_imports does, against your own empty_4.6.x and empty_4.7.x defaults and cross-checked against ArduPilot master; a second reviewer re-derived the same table independently.

This is inconsistent with the file's own convention rather than a deliberate choice — derived_parameters at 419-424 already lists ATC_ACCEL_{P,R,Y}_MAX and ATC_ACC_{P,R,Y}_MAX side by side under "if": "'X' in fc_parameters", and PSC_D_ACC* appears at 675-677, 758-759 and 1148. Broadening covers both: ATC_ACC(EL)?_[PRY]_MAX, (ATC_)?ANGLE_MAX, PSC_(ACCZ|D_ACC)_[IP], (WPNAV_|WP_).*, ATC_(SLEW_YAW|RATE_WPY_MAX).

Two smaller issues

• The second harmonic notch is never matched — configuration_steps_ArduCopter.json:699 (and pre-existing at :600). INS_HNTCH[0-9]*_.* cannot match INS_HNTC2_*: ArduPilot registers the second notch group with prefix _HNTC2_, not _HNTCH2_ (AP_InertialSensor.cpp:577-579), so the [0-9]* is dead. One of your own vehicle templates sets ten INS_HNTC2_* values in 25_motor_notch_filter_results.param and none are matched. Suggest INS_HNTC[H2]_.*.
• Two referenced screenshots are not committed — USERMANUAL.md:183 and :226 reference images/App_screenshot_Vehicle_directory_create_from_flight_controller.png and …_creator.png, neither of which exists under images/ at head. scripts/regenerate_app_screenshots_fully_automated.py was updated to generate them, but that does not supply the files, so both render broken on the docs site.

Notes

• re.match is start-anchored, so several patterns are broader than they look — configuration_steps_ArduCopter.json:649. ATC_RAT_(PIT|RLL|YAW)_[DIP] matches 18 names in both the 4.6 and 4.7 snapshots, not the 9 apparently intended — it also pulls _IMAX, _PDMX and _D_FF. Same for PSC_ACCZ_[IP] (4, not 2) and RTL_ALT (3, not 1). Appending $ gives exact matching where that is what you want. I deliberately did not raise cross-step duplication: 84 of 385 parameters in Holybro_X500_V2 already appear in more than one step file, so overlapping scopes are clearly normal here.
• Possible coverage gaps, offered as a suggestion not a requirement. Checking each new regex list against what your 17 real ArduCopter templates actually store in that step (excluding parameters the step already declares forced/derived): BRD_HEAT_TARG 16/17 — though the step's own why text says "set board target temperature", only BRD_HEAT_LOWMGN is imported; ATC_ANG_{RLL,PIT,YAW}_P 17/17 in steps 36/38/40, which autotune tunes and which neither ATC_RAT_* nor ATC_ACCEL_* covers; MOT_SPIN_MAX and TKOFF_RPM_MIN 17/17; LAND_ALT_LOW, PSC_ACCZ_SMAX, PSC_VELXY_FF 17/17; SCHED_LOOP_RATE, SCR_ENABLE, INS_POS{1,2}_{X,Y} 17/17, AUTO_OPTIONS 10/17.
• The two renamed buttons differ by one word — frontend_tkinter_project_opener.py:92 says "Create a vehicle project from a template", frontend_tkinter_project_creator.py:144 says "from template". TUNING_GUIDE_ArduCopter.md:155, TUNING_GUIDE_ArduPlane.md:155, TUNING_GUIDE_Heli.md:154 and TUNING_GUIDE_Rover.md:155 describe the creator but quote the opener's wording (USERMANUAL.md:219 is right). Also USERMANUAL.md:176 has a stray space: "flight controller is connected , you can".
• Copilot's invalid-JSON claim is wrong — the file parses at head and the comma it says is missing is present at line 671, visible even in Copilot's own displayed hunk. Flagging it so you do not go looking.
• I initially had a finding about the new strings being absent from the .pot and the five .po catalogues, and dropped it on cross-check: ai-translation.yml runs after Python changes reach master, so that is normal project automation, not something for you to do.

What was checked and came out clean

Autoimport cannot clobber user data — this was the first thing I looked for and it holds: data_model_configuration_step.py:180-201 skips anything already in current_step_parameters or parameters_to_delete and imports only values differing from the default, and autoimported parameters go through create_ardupilot_parameter(), which applies forced/derived rules, so an autoimport cannot defeat a forced value. The blocker above is a wrong baseline, not a broken precedence rule. Every other new regex was checked individually against empty_{4.5,4.6,4.7}.x and, where relevant, ArduPilot master — all correct on both 4.6 and 4.7, including several that looked odd in context (NTF_* in the ESC step, WPNAV_RADIUS/CAN_* in the GNSS step) but match exactly what real templates put in those step files. ParameterForcedOrDerivedError subclasses ValueError and is caught before the generic except (ValueError, TypeError) in data_model_parameter_editor.py:351-356 — ordering correct, no bare except, no behaviour change for other errors. _get_fc_template_dir_for_project_creation failure paths (no FC, missing/short/non-numeric version, no matching template) all raise a translated VehicleProjectCreationError, and the opener button is disabled unless an FC is connected with parameters. Data-model/view separation respected. Tests run locally: 464 passed across the four data-model suites, 79 passed / 2 skipped across the two frontend suites and the configuration-step suite.

CI is effectively green — 33 checks pass including all four pytest matrix jobs (4 956 tests, 0 failures). The two red checks are unrelated infrastructure: markdown-link-check fails on 403s from coveralls.io/blackduck.com plus three relative links in ARCHITECTURE.md, a file this PR does not touch; and Publish Tests Results reports "All 4 956 tests pass" with "conclusion": "success" before a downstream shell step in the same job fails.

Automated review note — AI-generated (Claude), independently cross-checked by a second model and re-verified against the live diff. Please sanity-check before acting.

Re-reviewed at head 0fba8236b4; my earlier comment above is superseded. Full report: follow-up report.

Verdict: REQUEST CHANGES — but for a different reason than last time. Everything I raised last round is resolved, and the blocker is proven fixed in both directions. An independent cold pass then found a new defect in the autoimport feature itself, and I have reproduced it: an autoimported parameter is never recorded as a change, so with the shipped default settings it is displayed in the editor and never written to disk.

Resolved since 58da5abb71

• The wrong "what is non-default" baseline → fixed in 013ee1986a (not the head commit): data_model_vehicle_project.py:313-314 snapshots param_default_dict before the re_init() at :319 and writes it at :323. 0fba8236b4 then fixes a second instance of the same root cause on the template path (:186, :191-193) — good catch, I had only reported the import path.

  scenario (real LocalFilesystem, factory-settings FC)	at head	ordering reverted
  FC defaults = Holybro_X500_V2 (1086 params, 138 differ from empty_4.6.x)	0 imported; 00_default.param == FC defaults (1086)	73 imported; 00_default.param = template's (1019)
  SERIAL5_BAUD=57, FC default 115, template default 57	00_default.param = 115.0, exported as 57.0 — retained	00_default.param = 57.0, SERIAL5_BAUD in no file — dropped

  Both directions confirmed, and both fixes are pinned: reverting 013ee1986a's ordering fails tests/test_data_model_vehicle_project.py:460 (assert 57.0 == 115.0) plus test_configured_fc_defaults_survive_destination_reinitialization; reverting 0fba8236b4's three lines fails :503. The MagicMock(spec=LocalFilesystem) gap is closed with real-filesystem tests, and no tests were lost in the refactor (that file went 61 → 70).

• All ten 4.6→4.7 renamed patterns → broadened exactly as suggested. Re-derived independently against your own empty_4.6.x (1019 params) and empty_4.7.x (1090): :369 (WPNAV_RADIUS|WP_RADIUS_M) 1/1, :398 ATC_ACC(EL)?_[PRY]_MAX 3/3, :468 (ATC_)?ANGLE_MAX 1/1, :485 ATC_(SLEW_YAW|RATE_WPY_MAX) 1/1, :663 PSC_(ACCZ|D_ACC)_[IP] 4/4, :986/:1017/:1051 1 each, :1332 1 and 12.

• The second harmonic notch → :600 and :699 are now INS_HNTC[H2]_.*, matching 17 of the 17 parameters in Big_Owl/25_motor_notch_filter_results.param, up from 8.

• Button wording and the stray space → frontend_tkinter_project_opener.py:92 and frontend_tkinter_project_creator.py:144 now agree, all four TUNING_GUIDE_*.md quote that wording, and "connected , you can" is gone.

BUG — an autoimported parameter is never written to disk with default settings

data_model_configuration_step.py:189-203 injects the imported parameter straight into current_step_parameters. The two sibling injection paths both record it as a change — forced parameters at data_model_parameter_editor.py:1954 and add_parameters at :1971 each do self._added_parameters.add(param_name) — but _apply_auto_imports records nothing.

It also cannot be is_dirty: it is built as Par(float(live_value), ""), so ArduPilotParameter.__init__ sets _value_on_file and _new_value to the same value (data_model_ardupilot_parameter.py:86, :103) and leaves the change reason equal to its on-file counterpart.

So _has_unsaved_changes() (:2284-2305) returns False, and the save gate at :462 reads has_param_changes = self._has_unsaved_changes() or annotate_params_into_files. That second term is the only thing that would save the value — and ProgramSettings ships "annotate_docs_into_param_files": False (backend_filesystem_program_settings.py:177), plumbed through frontend_tkinter_parameter_editor.py:481-482. Measured by driving your real _has_unsaved_changes and handle_write_changes_workflow:

The cold pass reached the same conclusion from the other end, with a real LocalFilesystem and empty_4.6.x: a non-default ATC_RAT_RLL_PDMX=0.5 appeared in the editor, the save workflow reported no changes, no prompt was shown, and the value was absent from disk.

Suggested fix: have _apply_auto_imports return the names it injected and add them to _added_parameters, exactly as the add_parameters path already does at :1971.

Why the tests miss it: every autoimport test exercises _apply_auto_imports in isolation and asserts only on the contents of current_step_parameters; none continues into the save workflow. Deleting the new quick-tune rule outright still leaves 548 tests passing.

Also worth fixing

• I/O failures in the new configured-FC workflow escape as untranslated tracebacks. create_new_vehicle_from_flight_controller (data_model_vehicle_project.py:202-250) raises a translated VehicleProjectCreationError from every guard clause, but calls _complete_imported_vehicle_project_creation with no try, and neither write_param_default_values_to_file (:323) nor export_to_param (:329) translates anything. Making the destination read-only reproduces a real PermissionError escaping the window callback with messagebox.showerror never called. Related, same handler: frontend_tkinter_project_creator.py:255-259 catches only VehicleProjectCreationError, but open_vehicle_directory (:336, :408-425) raises VehicleProjectOpenError — an unrelated Exception subclass — after the project has been written. The template path at :245-249 has the identical exposure so that half is not a regression, but this is new code and could catch both. This corrects my clean note from last round, where I said the FC-creation failure paths all raise a translated error: true of the guard clauses, not of the I/O.
• (re-raised) The two referenced screenshots are still not committed. USERMANUAL.md:183 and :226 reference images/App_screenshot_Vehicle_directory_create_from_flight_controller.png and …_creator.png; neither exists at head, and they are the only two broken image references in the file. f80d0c464b chore: regenerate application screenshots adds no image files at all — it touches only scripts/regenerate_app_screenshots_fully_automated.py and frontend_tkinter_project_opener.py. (The …_create_from_configured_{options,name,create}.png files that USECASES.md:155-159 uses do exist.) Note markdown-link-check did not run for this head — it is path-filtered and your last commit touches only .py — so CI will not catch these.
• One more instance of the rename class you just fixed: configuration_steps_ArduCopter.json:748 carries PSC_ACCZ_N[ET]F, which matches 2 names in 4.6 and 0 in 4.7 — they are PSC_D_ACC_NEF/PSC_D_ACC_NTF now, and your own empty_4.7.x/28_pid_notch_filter_results.param stores exactly those, so on 4.7 the vertical-accel notch values never autoimport into step 28. The line is pre-existing rather than added here. PSC_(ACCZ|D_ACC)_N[ET]F matches 2 on both.
• The notch fix stops one group short: INS_HNTC[H2]_.* misses INS_HNTC3_* and INS_HNTC4_*. ArduPilot registers all four groups (libraries/AP_InertialSensor/AP_InertialSensor.cpp:574, :579, :585, :591) and your own empty_4.7.x/00_default.param already contains INS_HNTC3_ENABLE. Measured: INS_HNTC[H2]_.* → 2 names in 4.7, INS_HNTC[H2-4]_.* → 3.

Notes

• (re-raised) re.match is start-anchored but not end-anchored, so several patterns stay broader than they look, re-derived at this head: :649 ATC_RAT_(PIT|RLL|YAW)_[DIP] → 18 in both 4.6 and 4.7 (not 9 — it also pulls _IMAX, _PDMX, _D_FF), :663 → 4, :485 RTL_ALT → 3. Also broad: RC[0-9]+_.+ 96, EK3_.* 83, SERVO[0-9]+_.+ 80. Appending $ gives exact matching where that is what you want. Note only — cross-step overlap is clearly normal in this file.
• The forced/derived warning drops the parameter name. data_model_parameter_editor.py:352 logs logging_warning("%s", exc), so the message names no parameter, unlike its siblings at :349 and :355. And if every relevant parameter is forced or derived, params_copied stays 0, _update_parameters_from_fc_values returns False, and the user gets no feedback at all after answering YES to the copy dialog (:377-388).
• Coverage gaps, partially addressed — suggestion only. Now covered: ATC_ANG_YAW_P (:398), SCR_ENABLE (:250), BRD_HEAT_TARG (via BRD_HEAT_.* at :14). Still matched by nothing, over your 21 ArduCopter template dirs: ATC_ANG_{RLL,PIT}_P 20/21, MOT_SPIN_MAX 20/21, TKOFF_RPM_MIN 20/21, SCHED_LOOP_RATE and INS_POS{1,2}_X 20/21, LAND_ALT_LOW and PSC_ACCZ_SMAX 19/21, PSC_VELXY_FF 19/21, AUTO_OPTIONS 13/21.
• Docs: USERMANUAL.md:232-236 does not mention that the FC's defaults are written to 00_default.param, nor that an xx_imported_flight_controller_parameters.param file may be created — both of which the analogous bin-log section (USERMANUAL.md:248-251, USECASES.md:119) documents.

Checked and clean

Autoimport still cannot clobber user data or defeat a forced/derived rule — re-checked against the new regexes with an adversarial FC set (empty_4.7.x defaults with every value perturbed so nothing looks default), driving the real _apply_auto_imports over every step file: 688 parameters autoimported across 25 step files, 0 existing step parameters modified, 0 forced or derived parameters added or overridden. The bug above is a missing change-record, not a broken precedence rule. 3404535d22's exception ordering is correct and pinned — ParameterForcedOrDerivedError is caught at :351-353, before except (ValueError, TypeError) at :354-356, and swapping them fails test_system_warns_without_a_traceback_when_a_parameter_is_forced_or_derived. _get_fc_template_dir_for_project_creation raises a translated error on every guard branch; the opener button is disabled unless an FC is connected and has parameters; data-model/view separation respected. I chased a hypothesis that :186 could snapshot stale defaults from a previously-open project and disproved it — the creator is only reachable when the vehicle dir has no parameter files, so param_default_dict is either the FC's defaults or empty, and empty is falsy so :191 skips. Local tests: 510 passed across the five relevant data-model suites, 36 passed / 2 skipped across the two frontend project suites.

Copilot's invalid-JSON claim is still wrong — the file parses at this head (63 steps) and the comma it says is missing is present at :671. Flagging again so you do not go looking.

CI: 30 pass / 1 skip / 1 fail. All four pytest matrix jobs green (5 174 tests, 0 failures). The failure is Publish Tests Results → Check coverage (coverage report --fail-under=89) against TOTAL 20156 2105 6116 607 88%; master at your base cf063929b6 gives 20091 … 88% and fails identically, as does master at the merge base — not yours. Separately worth raising with the maintainer: the repo's two coverage reporters disagree — the PR comment bot reports 90% (statement-only, 18051/20156 = 89.6%) while the gate uses coverage report, which includes branch coverage and yields 88%.

Automated review note — AI-generated (Claude), independently cross-checked by a second model and re-verified against the live diff. Please sanity-check before acting.

Re-reviewed at head 0f5af49259; my earlier comment above is superseded. Full report: https://uav.tridgell.net/DevCallReviews/followups/2026_09_10_2312/devcall_pr_reviews.html#prMethodicConfigurator-2047

The blocker is fixed, and it is the best-evidenced fix this PR has had — REQUEST CHANGES → COMMENT. Proven end-to-end with a real filesystem in both directions and pinned by three independent mutations against three named tests. Everything else from last round is resolved or reduced to a note. What keeps me off APPROVE is that the exception-translation finding is only partially closed, plus three small regex items.

Resolved

The blocker — autoimported parameters were never written to disk with the shipped defaults. data_model_configuration_step.py:152,181,210,212 now return the injected names and data_model_parameter_editor.py:1891 does _added_parameters.update(...) — exactly as suggested. With a real LocalFilesystem and ParameterEditor, empty_4.6.x, annotate_params_into_files=False (the shipped default), FC ATC_ANG_RLL_P=7.5 against a 4.5 default:

Mutation-proved three ways, each re-run end-to-end and against pytest:

Also resolved: both screenshots are committed (valid PNG data, 715×590 and 1192×335; no missing local image reference anywhere in the top-level .md files); PSC_ACCZ_N[ET]F → PSC_(ACCZ|D_ACC)_N[ET]F, now 2 on 4.6 and 2 on 4.7; the notch groups → INS_HNTC[H2-4]_.*, taking 4.7 from 2 to 3; ATC_RAT_(PIT|RLL|YAW)_[DIP]$ from 18 matches down to 9/9; and the docs now mention 00_default.param and the imported-parameters file (USERMANUAL.md:182-184, :240-241).

Coverage re-derived over all 24 ArduCopter template dirs (2371 distinct names): rule-matched names go 1234 → 1241, newly covering ATC_ANG_{RLL,PIT}_P, MOT_SPIN_MAX, TKOFF_RPM_MIN, SCHED_LOOP_RATE, INS_POS{1,2}_X, AUTO_OPTIONS, PSC_ACCZ_SMAX, PSC_D_ACC_{NEF,NTF}, INS_HNTC3_ENABLE and BRD_ALT_CONFIG. Nothing is lost against master's rules.

Partially resolved — SystemExit still escapes the new handler

ardupilot_methodic_configurator/data_model_vehicle_project.py:247-261

The OSError half is genuinely fixed — I drove a real OS PermissionError (destination chmod 0555) through create_new_vehicle_from_flight_controller and got a translated VehicleProjectCreationError; removing the try lets the raw PermissionError escape and fails test_configured_fc_creation_translates_persistence_errors. Good.

But the chain also includes a BaseException:

_complete_imported_vehicle_project_creation
  -> LocalFilesystem.re_init()                      backend_filesystem.py:94
       -> parse_parameter_metadata()                annotate_params.py:597-601
            -> get_xml_data() -> all URLs fail
                 -> raise SystemExit("Download failed.")   annotate_params.py:188-191

SystemExit derives from BaseException, so except (OSError, ParamFileError, ValueError, TypeError) cannot catch it. It escapes the window callback untranslated, and a SystemExit raised inside a Tkinter callback can take the app down. The offline / documentation-unreachable case is the most likely real failure for someone creating a project.

To be fair about it: this is not a regression. Before this delta the same call had no try at all, and re_init() raising SystemExit on a failed metadata download is pre-existing behaviour app-wide. You strictly improved things. That is why I've left this a partially-closed finding rather than a blocker — my second reviewer rated it a BUG and wanted REQUEST CHANGES, and I've not followed that.

Worth fixing — two regex items on one line

ardupilot_methodic_configurator/configuration_steps_ArduCopter.json:663, PSC_ACCZ_(IP|SMAX)$

1. (IP|SMAX) is a literal alternation, not a character class. It means PSC_ACCZ_IP or PSC_ACCZ_SMAX, and PSC_ACCZ_IP exists in no ArduPilot version — zero hits across all 2371 distinct names in every ArduCopter template dir. The old PSC_(ACCZ|D_ACC)_[IP] matched PSC_ACCZ_I and PSC_ACCZ_P; this doesn't. Comparing old and new matched-name sets, the exact loss is PSC_ACCZ_I, PSC_ACCZ_P (unintended — the typo) plus PSC_ACCZ_IMAX, PSC_ACCZ_PDMX, PSC_D_ACC_IMAX, PSC_D_ACC_PDMX (intended — the $ doing its job). Impact is limited since PSC_ACCZ_I/_P are already in the 4.5/4.6 24_throttle_controller.param step file, so autoimport skips them anyway — but the rule has a dead branch. Suggest PSC_ACCZ_([IP]|SMAX)$.
2. The 4.7 counterpart of the SMAX gap you just closed is still open. PSC_ACCZ_SMAX is now covered for 4.6, but its 4.7 rename PSC_D_ACC_SMAX is matched by nothing — PSC_D_ACC_[IP]$ excludes it, and it's in empty_4.7.x/00_default.param and not in that version's step file. Net effect of the whole rule on 4.7 is 0 names. Step 46 already handles this class correctly by listing both spellings. Suggest PSC_D_ACC_([IP]|SMAX)$.

Notes

• RTL_ALT$ matches 0 names on 4.7 — and this one is mine to own. configuration_steps_ArduCopter.json:485. Measured against the ArduCopter templates: 4.5 → 1, 4.6 → 1, 4.7 → 0, because 4.7 renames it RTL_ALT_M (and RTL_ALT_FINAL → RTL_ALT_FINAL_M). That's a direct consequence of my advice last round: I pointed out re.match isn't end-anchored and that RTL_ALT matched three names, and adding $ is right for 4.6 and wrong for 4.7. No net coverage is lost — step 15's RTL_.* picks up RTL_ALT_M — so this only means the safety step never autoimports the RTL altitude on 4.7 while it does on 4.6. RTL_ALT(_M)?$ restores the symmetry. Sorry for the round trip.
• Dropping BRD_ALT_CONFIG,0 from the 4.5.x/4.6.x step-06 templates removes it from the guided sequence when it's at its default. The autoimport replacement only fires when the live FC value differs from param_default_dict (data_model_configuration_step.py:199-210), so a user working from empty_4.6.x with no FC connected, or with BRD_ALT_CONFIG still 0, no longer sees the parameter at all — where before it was listed with value 0. It appears in no .md, so there's no documentation fallback. The 29 other non-empty templates still carry it. Deliberate?
• (still open) The forced/derived warning still drops the parameter name — data_model_parameter_editor.py:352 is logging_warning("%s", exc), unlike :349/:355 — and :391-397 still shows nothing when params_copied == 0.
• create_new_vehicle_from_bin_log (data_model_vehicle_project.py:404) still calls _complete_imported_vehicle_project_creation with no try, while its docstring says Raises: VehicleProjectCreationError. It's the sibling of the path you just fixed, and this PR refactored it, so it's cheap to give it the same wrapper.
• The template-path VehicleProjectOpenError catch at frontend_tkinter_project_creator.py:249 has no test — replacing both clauses with the old single-exception form fails only the FC-path test.
• Passing autoimported_parameters via instance state rather than process_configuration_step's return tuple is a little fragile: a direct _apply_auto_imports caller leaves it stale, and an exception between :148 and :152 leaves the previous step's set in place. Harmless today; the return tuple would be more obvious.
• Three images are now orphaned by the USECASES.md rewrite: App_screenshot_Vehicle_directory_create_from_configured_{options,name,create}.png.

Checked and cleared

Autoimport still cannot clobber user data or defeat a forced/derived rule, re-checked against the NEW regexes. Adversarial FC set with every default perturbed, driving the real ConfigurationStepProcessor over all 63 steps: empty_4.6.x → 638 autoimported, 0 existing step parameters overwritten; empty_4.7.x → 668, 0. The one collision flagged — RC5_OPTION in step 07, both RC[0-9]+_.+-matched and derived-on-ExpressLRS — I chased end to end with a real LocalFilesystem and a live RC5_OPTION=42: the derived rule wins cleanly, and the derived value is now actually persisted, which it would not have been before this fix. All 125 rules re-counted against both firmware snapshots; every zero-match rule inspected, and the remaining zeros are all conditionally-registered subtrees absent from a stock 00_default.param. Copilot's invalid-JSON claim is still wrong — the file parses at this head, 63 steps. Local suite: 5042 passed, 50 skipped, 4 xfailed.

CI at this head: 28 unique check-runs reconciled against total_count — 17 passing, 1 failing, 10 still in flight. The failure is markdown-link-check and it is not yours, verified two ways: the workflow checks every .md in the repo, and the 9 files with dead links (COMPLIANCE.md, SECURITY.md, ARCHITECTURE.md, ROADMAP.md, CONTRIBUTING.md, README.md, GUIDE_log_analysis.md, ARCHITECTURE_log_analysis.md, LICENSE.md) are none of the ones you touched — all seven PR-touched .md files report zero dead links — and the workflow has concluded failure on every one of its last 20 runs including master. This supersedes my remark last round that the job was path-filtered out; it runs now, because .md files changed. The Check coverage gate remains pre-existing on master as established previously.

Verdict: COMMENT

• RESOLVED — The blocker — autoimported parameters never written to disk with the shipped defaults. Still fixed, and now refactored: data_model_configuration_step.py:153,174-181 returns autoimported_parameters as the 7th element of the result tuple and data_model_parameter_editor.py:1912 does self._added_parameters.update(autoimported_parameters). grep finds exactly one production caller of process_configuration_step, and all 12 mocked return values in tests/test_data_model_parameter_editor.py were widened to 7-tuples.
• RESOLVED — SystemExit escaped the new handler. SystemExit is now in the except tuple at data_model_vehicle_project.py:201, :258 and :415. VERIFIED BY EXECUTION, both directions: a real SystemExit("Download failed.") raised from LocalFilesystem.re_init inside create_new_vehicle_from_flight_controller is translated to VehicleProjectCreationError at this head, and escapes untranslated when the clause is removed. The chain was re-read — re_init is called at :331 and :346 inside _complete_imported_vehicle_project_creation, both now inside a guarded caller — and VehicleProjectOpenError is deliberately NOT in the tuple so the creator window still displays it. (See New findings for the coverage gap.)
• RESOLVED — PSC_ACCZ_(IP|SMAX)$ was a literal alternation. configuration_steps_ArduCopter.json:663 is now PSC_ACCZ_([IP]|SMAX)$. Measured against 00_default.param: 4.5.x and 4.6.x go 1 -> 3 matches (PSC_ACCZ_I, PSC_ACCZ_P, PSC_ACCZ_SMAX); PSC_ACCZ_IMAX/PSC_ACCZ_PDMX stay excluded, as intended.
• RESOLVED — The 4.7 SMAX counterpart. Same line, PSC_D_ACC_([IP]|SMAX)$. 4.7.x goes 2 -> 3, picking up PSC_D_ACC_SMAX.
• RESOLVED — RTL_ALT$ matched 0 names on 4.7 (this review's own bad advice last round). configuration_steps_ArduCopter.json:485 is now RTL_ALT(_M)?$: 4.7 goes 0 -> 1 (RTL_ALT_M), 4.5/4.6 unchanged at 1. Full old-vs-new coverage diff over all 143 regex rules in 39 of 63 steps: 4.5.x 598->600, 4.6.x 592->594, 4.7.x 680->681, NOTHING LOST in any version.
• RESOLVED — The forced/derived warning dropped the parameter name. data_model_parameter_editor.py:357 now formats "Parameter {parameter} could not be updated because it is forced or derived: {error}", and tests/test_data_model_parameter_editor.py:6207 pins the exact string. :350 and :364 got the same treatment.
• RESOLVED — Nothing shown when params_copied == 0. data_model_parameter_editor.py:409-412 adds a "No parameters copied" show_info, correctly nested under if user_choice is True. See New findings for two caveats.
• RESOLVED — create_new_vehicle_from_bin_log had no try. data_model_vehicle_project.py:404-419 now wraps _complete_imported_vehicle_project_creation with the same handler as its two siblings.
• RESOLVED — Instance state instead of a return tuple. Done exactly as suggested, in its own commit (790bd74 "refactor(config): return auto-imports explicitly"). grep -rn autoimported_parameters finds no surviving attribute reference anywhere in the package.
• RESOLVED — Three orphaned images. All three PNGs deleted, and their CaptureTarget entries removed from scripts/regenerate_app_screenshots_fully_automated.py. Zero references remain in any .md, .py or .json, and every images/*.png link in the top-level .md files resolves to an existing file.

• verified — ardupilot_methodic_configurator/data_model_parameter_editor.py:409-412 — the new "No parameters copied" branch is untested, and its wording covers two different situations. Deleting the whole else: block changes nothing in tests/test_data_model_parameter_editor.py + tests/test_frontend_tkinter_parameter_editor.py (both runs: 9 failed / 382 passed, and the 9 are identical pre-existing tkinter-display failures in this headless environment). The three existing handle_copy_fc_values_workflow tests at :3904, :3927, :3946 all stub _update_parameters_from_fc_values to True or never reach it. Separately: that method returns False both when every value failed AND when every FC value was already within tolerance (ParameterUnchangedError, data_model_ardupilot_parameter.py:469), so a user who re-enters a step they already copied gets "No FC values could be copied to X." for what is actually "already up to date". Worth splitting the two messages.
• verified — Cross-step duplication is a property of the autoimport design, but it matches existing practice. NUMBERS CORRECTED by the cross-check, which recounted and did not reproduce this review's figures: counting regex candidates present in that version's defaults, absent from the destination file, and present in another numbered non-default file gives 112 (4.6.x) and 118 (4.7.x) distinct names — or 125/140 destination-step/name pairs — not the 172/188 first reported. The example was also wrong in detail: step 40 imports the YAW parameters specifically, not all three axes. The shape of the observation stands (a name can be a candidate for one step while another step's .param file already owns it, e.g. step 15's RTL_ALT_M vs step 16), but the magnitude was overstated. They only materialise when the live FC value is non-default, and derived parameters still win (derived_params_to_apply is computed at data_model_configuration_step.py:110-117, before _apply_auto_imports at :153, and applied by the editor afterwards). The shipped templates already duplicate parameters across steps deliberately, and tests/test_vehicle_template_param_files.py only asserts INTRA-file uniqueness — so this is recorded, not objected to. The hazard is the familiar one: editing an earlier file later and re-uploading a stale later file.
• verified — Not a finding, recorded so nobody else raises it. None of the five new/changed msgids exist in locale/ardupilot_methodic_configurator.pot, and the old %s form is still translated in de/it/ja/pt/zh_CN. That is expected: .github/workflows/ai-translation.yml regenerates the .pot and merges .po files on every push to master touching **/*.py. The only cost is orphaned translations — and the cross-check counted them properly: TWO replaced %s msgids each have translations in all five languages, so ten translated entries go stale, not five.